Hello @DanRyanIrish! Thanks for updating this PR.

• In the file ndcube/wcs/wrappers/resampled_wcs.py:

Line 1949:5: E129 visually indented line with same indent as next logical line
Line 1948:43: W504 line break after binary operator
Line 1947:40: W504 line break after binary operator
Line 1814:13: E129 visually indented line with same indent as next logical line
Line 1813:33: W504 line break after binary operator
Line 1317:52: W504 line break after binary operator
Line 1303:70: W504 line break after binary operator
Line 1299:44: W504 line break after binary operator
Line 877:13: E129 visually indented line with same indent as next logical line
Line 876:38: W504 line break after binary operator
Line 129:13: E129 visually indented line with same indent as next logical line
Line 128:41: W504 line break after binary operator

Comment last updated at 2021-08-06 09:40:05 UTC

@Cadair, here are my changes to ResampledWCS. Tests still need fixing. But before I invest that effort, any comments on what I'm doing?

I haven't sat down and worked through the maths of the translation (do you want me to?), but looks good otherwise.

A check on the maths would be welcome but not expected.

In going through the tests, I've come across an important point regarding pixel_bounds. My understanding is that pixel_bounds refer to the center of the pixels at either end of the limit of the axis. Currently, ResampledWCS just translates the centre of the underlying pixel_bounds to the new pixel grid. However, this does not represent that limit of the new pixel grid for which the transformations are defined.

Consider a 1-D pixel grid that we upsample by a factor of 1/3, i.e. there are 3 new pixels for every old/underlying pixel. Say the pixel bounds were (-1, 2). The location of the centre of these pixels in the new pixel grid is (-3, 6). However, the full extent of the defined range includes the edges of these pixels. Defined with pixel edges, the pixel_bounds (-1.5, 2.5). In the new pixel grid, this corresponds to (-4.5, 7.5). This means that in the new pixel grid, pixels -4 -- 7 (inclusive) are fully within the pixel bounds of the original pixel grid. However, pixel_bounds currently returns (-3, 6), implying that the transformations aren't defined for new pixels -4 or 7.

In the above example I chose numbers that worked out nicely in whole pixels. But this might not always be the case. We must therefore clarify what pixel_bounds represents. Is it:

• the centre of the last whole pixel included in the valid range? (If this is the case then resampling can cause the valid range in world coords to shrink a little if a non-integer number of new pixels fit into the pixel bounds.)
• the centre the the last pixel that has at least some of its area in the valid range? (If this is the case then resampling can cause the valid range in world coords to increase a little for the same reason as above.)
• the non-integer pixel value representing the edge of the valid range. If this is the case, the pixel bounds can always be kept the same in world coords between the new and old pixel grids.

According to @ayshih the answer to the above is option 3.

Any further comments before merging this @Cadair? (The failing test will be resolved once #451 is merged.)

Hi @dhomeier. Thanks so much for your comments. I've taken them into account and will add an extra test as you suggest.